Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(coverage-recipe): add front end coverage for jest #401

Merged
merged 1 commit into from
Apr 7, 2022

Conversation

FloValdes
Copy link
Contributor

Context

Potassium uses Jest for testing frontend code when using Vue. Jest provides some options to report test coverage.

Changes

Change the coverage recipe to add the necessary setup for Jest test coverage (if Jest is used). It modifies the existing package.json to add the coverage configuration.

@FloValdes FloValdes requested a review from difernandez April 5, 2022 20:24
@@ -32,4 +44,18 @@ def configure_rails_helper
end
end
end

def add_coverage_config(js_package)
js_package['scripts']['test'] = 'jest --changedSince=master'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acá con esto del --changedSince=master me entra la duda. Esto haría que siempre que se corra yarn test, solo se testeen los archivos con cambios con respecto a master. Pero por lo que entiendo, ese comando debería siempre correr todos los tests. Lo que se tiene que limitar a que sea solo con respecto a los cambios de master es lo que revise el linter (lo que iría en la otra tarjeta). Osea creo que esto debería mantenerse con el valor anterior, test: "jest" y cuando se vea lo de CI/linter habría que ver una manera de que al correr eso se genere un reporte que considere solo los cambios de master.
No estoy seguro si, así que invoco a @ldlsegovia pa que me corrija

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hola, vengo de 🐸. Hay un par de proyectos que usan main en vez de master, habría que manejar ese caso también 🤔

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Si el reporte es un html que uno entra a ver, deberían incluirse todos los archivos.
  • Si el reporte es el que se muestra en la terminal, luego del correr los tests, mejor que sea con el diff con master para que uno sepa a qué no le agregó tests
  • En CI, también con el diff para que el mono no mate a comentarios.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ldlsegovia claro, pero el yarn test debería correr todos los tests, solo que cambia el scope dependiendo del reporte que se genere, eso?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gmq mmm se podrá considerar ese caso? quizás hay que cambiar esa configuración manualmente si se decide ir por main 🤔 en otros lados tambn se asume que es master si, o acá al menos

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Toda la razón, ignórenme :shame:

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

está el problema de que no es tan customizable de qué tests hacer coverage, se hacen automáticos de los que se corren nomás, entonces quizás una solución sería tener dos comandos, seguir con el yarn test como está y agregar uno que solo vea tus cambios y genere el coverage, y sea ese último el que el mono revise para molestarte

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suena bien eso, démosle con tener los dos comandos separados. Después cuando se haga lo de CI se pueden correr dos veces los tests ahí, una para los tests corriéndolos todos, y otra corriendo solo con los cambios desde master para el linting.

Y duda: como funciona el reporter usando yarn test:watch? Te va mostrando un reporte del archivo que se va corriendo?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sí, con yarn test:watch te muestra el coverage de los archivos que se van corriendo, funciona igual que un --onlyChanged

Copy link
Contributor

@difernandez difernandez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@FloValdes FloValdes merged commit ad8a741 into master Apr 7, 2022
@FloValdes FloValdes deleted the coverage-front branch April 7, 2022 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants